Gergeljkis/azure vnet injection terraform #10
Conversation
|
Hi all, Here's what's happening. When this code deploys the workspace, the Azure identity is assigned as the workspace admin. This identity is typically the I wanted to keep the code as simple as possible, without additional user provisioning/managing. That's why I haven't yet explored possible solutions for this. Let me know what your thoughts are on this. (cc @haleyyyblue) |
haleyyyblue
left a comment
haleyyyblue
left a comment
There was a problem hiding this comment.
Hey @gergelj ! Thanks for putting this together. The code is clean and well-organized. Here's my feedback:
🎉 What's working well
Great structure: The flat, purpose-specific file organization (azure.tf, databricks.tf, network.tf) aligns perfectly with our philosophy of keeping things simple and self-contained
Clear documentation: The README provides good step-by-step guidance for users
Consistent approach: Both scenarios follow the same patterns, making it easy to understand
🔧 Key suggestions
- NAT Gateway (Critical)
Azure will remove default outbound connectivity next year. We should explicitly add NAT Gateway resources now to future-proof this. Both public and private subnets should be configured with no_public_ip = true and explicit NAT Gateway associations. - Consider merging the two projects (High priority)
Looking at our aws-byovpc example, it handles both "new VPC" and "existing VPC" scenarios in a single project using conditional logic (vpc_id == "" ? create : use_existing). This approach:
Reduces duplication
Gives users one place to start
Makes maintenance easier
Would you be open to combining azure-vnet-injection-existing-vnet and azure-vnet-injection-new-vnet into a single azure-vnet-injection project with a conditional variable? - Remove CMK commented code
The commented-out CMK variables and configuration should be removed entirely from this scenario. CMK would be better as a separate scenario (e.g., azure-vnet-injection-cmk) following our scenario-based philosophy. This keeps each example focused and easier to understand. - Add versions.tf file
The AWS example includes explicit provider version constraints for reproducibility. Let's add a versions.tf to the Azure projects as well. - Additional improvements for consistency
To match the completeness of the AWS example:
More outputs: Add network details (subnet IDs, NSG ID, NAT Gateway ID) and resource group info
Variable validation: Add storage account name format, and CIDR ranges
Consider Unity Catalog: The AWS example includes metastore setup - might be worth adding here too
📝 Action items
Must have (blocking):
[ ] Add NAT Gateway resources with explicit outbound configuration
[ ] Add versions.tf with provider version constraints
[ ] Remove all CMK-related commented code (variables, configuration)
Should have (strongly recommended):
[ ] Merge the two projects into one with conditional VNet creation
[ ] Expand outputs to include network and resource details
[ ] Add variable validation
Nice to have:
[ ] Unity Catalog metastore support
[ ] Enhanced documentation with troubleshooting section
Let me know if you'd like to discuss any of these suggestions! Happy to pair on the NAT Gateway implementation or the project consolidation if that would help. 😊
|
Completed the following action items so far: Must have (blocking):
Should have (strongly recommended):
Nice to have:
These changes are applied to the If the NAT gateway script is alright, I would like to consolidate the (cc. @haleyyyblue) P.S. I would like to know the correct Azure CLI authentication method. I still get the |
|
@gergelj One small suggestion: Regarding the Azure CLI authentication issue, it might be helpful to ask in the #field-infra channel, as they can probably guide you more quickly and accurately. Also, consolidating the two directories sounds good to me once we finalize the NAT configuration. Thanks again for the great updates — really appreciate the effort! 🚀 |
Pull Request
Description
Terraform code for a simple Azure Databricks deployment with VNet injection.
There is one directory with code for deploying a workspace to an existing VNet and one that provisions a new VNet.
Category
Type of Change
Project Details
Project Name: Azure VNet injection Workspace Setup Guide (Terraform)
Purpose: Customer enablement
Technologies Used: Terraform, Databricks, Azure
Testing
Security Compliance ✅
By submitting this PR, I confirm I have followed the CONTRIBUTING.md guidelines and security requirements.